943 Local Setup - Kept Old US Local Setup In place - #1344
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new “Locale Setup” utility under the Utils tab to help users configure a UTF-8 locale (intended to address Debian’s default 12-hour time/locale experience discussed in #943), while keeping the existing “US Locale Setup” utility available.
Changes:
- Added a new Utils menu entry for “Locale Setup” pointing to a new
locale-setup.shscript. - Introduced
core/tabs/utils/locale-setup.shto detect/suggest a locale and generate it. - Updated the user guide walkthrough to mention the new utility.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| docs/content/userguide/walkthrough.md | Adds “Locale Setup” to the walkthrough list (currently with a small Markdown formatting issue). |
| core/tabs/utils/tab_data.toml | Registers the new “Locale Setup” utility in the Utils catalog (task flaging needs adjustment for privileged edits). |
| core/tabs/utils/locale-setup.sh | New script to suggest and apply a UTF-8 locale (needs hardening for idempotence, clearer failure behavior, and safer network call). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…timeout for iso lookup and defaulted to US if fails, addd conditional statements for modifying locale based on Copilot suggestions
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an interactive locale setup script that detects locale suggestions, validates input, updates system locale files, runs ChangesLocale Setup Utility
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant LocaleSetup as locale-setup.sh
participant SystemFiles as /etc locale files
participant LocaleGen as locale-gen
LocaleSetup->>User: Present locale choices
User-->>LocaleSetup: Select or enter locale
LocaleSetup->>SystemFiles: Update locale.gen and locale.conf
LocaleSetup->>LocaleGen: Generate selected locale
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
core/tabs/utils/locale-setup.sh (1)
12-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
printfinstead ofecho.Replace the locale-map and error-message
echocalls withprintffor portable shell output. As per coding guidelines, “useprintfrather thanechofor portable formatted output.”Also applies to: 72-72, 90-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/tabs/utils/locale-setup.sh` around lines 12 - 46, Replace every echo invocation in the locale mapping, fallback, and associated error-message paths with portable printf calls, including the locations referenced around lines 72 and 90. Preserve each existing locale value, fallback behavior, and error-message content while ensuring formatting remains equivalent.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/tabs/utils/locale-setup.sh`:
- Around line 12-46: Correct the country-to-locale mappings in the locale
selection case statement: use `gb` for `en_GB.UTF-8` instead of `uk`, map `ch`
to the appropriate Swiss locale instead of falling back, map `sg` to Singapore’s
locale rather than `de_CH.UTF-8`, and update `cf` to the Central African
Republic locale. Preserve all other mappings and the existing default fallback.
- Around line 61-76: Harden the LOCALE validation loop in locale setup to accept
C.UTF-8, require a safe locale-character whitelist that excludes
command-injection metacharacters, and verify the locale is available before any
/etc/environment update. Preserve the existing format validation for valid UTF-8
locales, then only pass validated values to the escalated sed commands.
In `@core/tabs/utils/tab_data.toml`:
- Around line 151-155: Add a command_exists precondition to the Locale Setup
entry in the tab data, checking for locale-gen before displaying or running
locale-setup.sh; follow the existing utility entries’ condition syntax and leave
the task metadata unchanged.
---
Nitpick comments:
In `@core/tabs/utils/locale-setup.sh`:
- Around line 12-46: Replace every echo invocation in the locale mapping,
fallback, and associated error-message paths with portable printf calls,
including the locations referenced around lines 72 and 90. Preserve each
existing locale value, fallback behavior, and error-message content while
ensuring formatting remains equivalent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d497e908-958e-40f5-af9e-b88ccd0f11ec
📒 Files selected for processing (3)
core/tabs/utils/locale-setup.shcore/tabs/utils/tab_data.tomldocs/content/userguide/walkthrough.md
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a729863eaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "$LOCALE" in | ||
| *[![:alnum:]_.@-]*|'') ;; | ||
| *.UTF-8) break ;; | ||
| esac |
There was a problem hiding this comment.
For any syntactically plausible but unsupported input such as en_UK.UTF-8, this check accepts the value, after which the script writes it to /etc/environment, /etc/locale.gen, and /etc/locale.conf before locale-gen reports failure. The failed run therefore leaves the system configured with an invalid locale; verify the value against the installed locale definitions before modifying any files.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/tabs/utils/locale-setup.sh`:
- Around line 9-10: Update the locale selection and validation logic around
suggested_locales to derive choices from /usr/share/i18n/SUPPORTED (or an
equivalent supported-locale list), accepting only validated full UTF-8
identifiers and special-casing C.UTF-8. Keep each source filename separate from
its generated locale identifier, reject traversal or arbitrary files in custom
input, and ensure numeric selections and LANG values retain the .UTF-8 suffix.
- Around line 42-50: Update the numeric validation in the choice-handling case
statement to reject zero before the sed lookup, while continuing to accept
positive integer selections. Preserve the existing “Invalid selection.” response
and avoid invoking sed for a zero choice.
- Around line 6-7: Validate iso against the exact two-letter pattern
^[A-Za-z]{2}$ before filtering, and update the suggested_locales logic to search
localeNames for an exact territory match such as _CA rather than using an
unanchored grep. Ensure invalid or empty responses produce no suggested locales.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c0bddf18-0085-47cd-9a1c-9219543356e6
📒 Files selected for processing (2)
core/tabs/utils/locale-setup.shcore/tabs/utils/tab_data.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- core/tabs/utils/tab_data.toml
| iso=$(curl -4fsSL --max-time 5 https://ifconfig.io/country_code 2>/dev/null) || iso="US" | ||
| suggested_locales=$(ls /usr/share/i18n/locales/ | grep -i "$iso" || true) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -e
printf 'Repository files matching locale-setup.sh:\n'
fd -a 'locale-setup\.sh$' . || true
file="$(fd 'locale-setup\.sh$' . | head -n 1 || true)"
if [ -n "$file" ]; then
printf '\nFile: %s\n' "$file"
wc -l "$file"
cat -n "$file"
fi
printf '\nRelevant grep/iso usages in core/tabs/utils/locale-setup.sh:\n'
if [ -n "$file" ]; then
rg -n -C 3 'iso=|ifconfig|country_code|suggested_locales|grep|locales/' "$file" || true
fi
printf '\nSample grep behavior with CA on common locale filename set:\n'
python3 - <<'PY'
from pathlib import Path
samples = sorted({
"ca_ES.utf8",
"ca_ES@euro",
"CA",
"usa",
"en_US.utf8",
"de_DE.utf8",
"us",
"CA",
})(
name for name in [
"ca_ES.utf8", "ca_ES@euro", "CA", "usa", "en_US.utf8", "de_DE.utf8", "us", "CA",
]
)
for pattern in ["", "CA", "CA\\b"]:
print("pattern", repr(pattern), ":")
for name in samples:
matched = True
try:
import subprocess
res = subprocess.run(["grep", "-i", pattern, name], input=name+"\n", text=True, capture_output=True)
matched = res.returncode == 0
except Exception as e:
matched = False
print(name, matched)
PYRepository: ChrisTitusTech/linutil
Length of output: 5204
🏁 Script executed:
#!/bin/sh
set -e
printf 'Repository files matching locale-setup.sh:\n'
fd 'locale-setup\.sh$' . || true
file="$(fd 'locale-setup\.sh$' . | head -n 1 || true)"
if [ -n "$file" ]; then
printf '\nFile: %s\n' "$file"
wc -l "$file"
cat -n "$file"
fi
printf '\nSample grep behavior with CA on common locale filename set:\n'
python3 - <<'PY'
import subprocess
samples = sorted([
"ca_ES.utf8",
"ca_ES@euro",
"CA",
"usa",
"en_US.utf8",
"de_DE.utf8",
"us",
"CA",
])
for pattern in ["", "CA", "CA\\b"]:
print("pattern", repr(pattern), ":")
for name in samples:
result = subprocess.run(["grep", "-i", pattern, name], input=name + "\n", text=True, capture_output=True)
print(name, result.returncode == 0)
PYRepository: ChrisTitusTech/linutil
Length of output: 3681
Validate the country code before filtering locales.
iso comes from an HTTP response and is used unanchored in grep. Use ^[A-Za-z]{2}$ and search localeNames for an exact territory match such as _CA, so false positives like ca_* and empty iso values do not enter suggested_locales.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/tabs/utils/locale-setup.sh` around lines 6 - 7, Validate iso against the
exact two-letter pattern ^[A-Za-z]{2}$ before filtering, and update the
suggested_locales logic to search localeNames for an exact territory match such
as _CA rather than using an unanchored grep. Ensure invalid or empty responses
produce no suggested locales.
| if [ -z "$suggested_locales" ]; then | ||
| suggested_locales=$(ls /usr/share/i18n/locales/) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use validated full UTF-8 locale identifiers.
The fallback lists every file in /usr/share/i18n/locales, and the custom branch accepts any regular file below that directory. A path such as ../../../etc/passwd can therefore pass the file check when the target exists, but it is not a locale. The code also rejects standard inputs such as en_GB.UTF-8 and C.UTF-8 unless matching source files exist. Numeric choices produce bare names such as en_GB, which later become LANG=en_GB.
Build choices and custom validation from /usr/share/i18n/SUPPORTED or an equivalent supported-locale list. Special-case C.UTF-8. Keep the source filename separate from the generated locale identifier. Debian documents /usr/share/i18n/SUPPORTED as the supported-locale list and uses full .UTF-8 values for locale configuration. (manpages.debian.org)
Suggested input model
- suggested_locales=$(ls /usr/share/i18n/locales/ | grep -i "$iso" || true)
+ supported_locales=$(awk '$2 == "UTF-8" { print $1 }' /usr/share/i18n/SUPPORTED)
+ suggested_locales=$(printf '%s\n' "$supported_locales" |
+ grep -iE "(^|_)${iso}([.@]|$)" || true)
if [ -z "$suggested_locales" ]; then
- suggested_locales=$(ls /usr/share/i18n/locales/)
+ suggested_locales=$supported_locales
fi
...
- if [ -f "/usr/share/i18n/locales/$custom_locale" ]; then
+ if [ "$custom_locale" = "C.UTF-8" ] ||
+ printf '%s\n' "$supported_locales" |
+ grep -qxF "$custom_locale"; thenAlso applies to: 24-40, 48-52
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/tabs/utils/locale-setup.sh` around lines 9 - 10, Update the locale
selection and validation logic around suggested_locales to derive choices from
/usr/share/i18n/SUPPORTED (or an equivalent supported-locale list), accepting
only validated full UTF-8 identifiers and special-casing C.UTF-8. Keep each
source filename separate from its generated locale identifier, reject traversal
or arbitrary files in custom input, and ensure numeric selections and LANG
values retain the .UTF-8 suffix.
| case "$choice" in | ||
| ''|*[!0-9]*) | ||
| printf "%s\n" "Invalid selection." | ||
| exit 1 | ||
| ;; | ||
| esac | ||
| LOCALE=$(printf "%s\n" "$suggested_locales" | sed -n "${choice}p") | ||
| if [ -z "$LOCALE" ]; then | ||
| printf "%s\n" "Invalid selection." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
echo "== locate file =="
fd -a 'locale-setup.sh' . || true
echo "== relevant file excerpt =="
if [ -f core/tabs/utils/locale-setup.sh ]; then
nl -ba core/tabs/utils/locale-setup.sh | sed -n '1,90p'
fi
echo "== sed on input 0p =="
printf 'line\n' | sed -n '0p' || true
printf 'line\n' | sed -n '1p' || true
echo "== sed availability =="
sed --version | head -n 1 || trueRepository: ChrisTitusTech/linutil
Length of output: 424
🏁 Script executed:
#!/bin/sh
set -eu
echo "== relevant file excerpt =="
if [ -f core/tabs/utils/locale-setup.sh ]; then
awk '{printf "%3d\t%s\n", NR, $0}' core/tabs/utils/locale-setup.sh | sed -n '1,120p'
fi
echo "== shell behavior for sed 0p under set -e =="
tmp="$(mktemp)"
trap 'rm -f "$tmp"' EXIT
printf 'line\n' > "$tmp"
(
set -e
sed -n '0p' "$tmp"
status=$?
printf 'sed status: %s\n' "$status"
)
status=${status:-0}
printf 'outer status: %s\n' "$status"Repository: ChrisTitusTech/linutil
Length of output: 3027
Reject 0 before the sed lookup.
The menu uses 1-based numbering, but the current validation allows 0. sed -n '0p' fails before reaching the empty-result check, and because this script runs with set -e, the selection cannot print “Invalid selection.” Reject zero as an invalid choice.
Minimal fix for zero
- ''|*[!0-9]*)
+ ''|0|*[!0-9]*)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "$choice" in | |
| ''|*[!0-9]*) | |
| printf "%s\n" "Invalid selection." | |
| exit 1 | |
| ;; | |
| esac | |
| LOCALE=$(printf "%s\n" "$suggested_locales" | sed -n "${choice}p") | |
| if [ -z "$LOCALE" ]; then | |
| printf "%s\n" "Invalid selection." | |
| case "$choice" in | |
| ''|0|*[!0-9]*) | |
| printf "%s\n" "Invalid selection." | |
| exit 1 | |
| ;; | |
| esac | |
| LOCALE=$(printf "%s\n" "$suggested_locales" | sed -n "${choice}p") | |
| if [ -z "$LOCALE" ]; then | |
| printf "%s\n" "Invalid selection." |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/tabs/utils/locale-setup.sh` around lines 42 - 50, Update the numeric
validation in the choice-handling case statement to reject zero before the sed
lookup, while continuing to accept positive integer selections. Preserve the
existing “Invalid selection.” response and avoid invoking sed for a zero choice.
Type of Change
Description
Added option for new Local Setup (UTF-8 only). Kept original US locale setup in place.
Issues / other PRs related
Screenshots (if applicable)